Clean up min block scripts - #429
Conversation
Make slp_eval and measure_e2e both easier to use wrappers around one common script.
e01bda0 to
f051651
Compare
There was a problem hiding this comment.
Pull request overview
Refactors MinBlockTimeMixed benchmark wrappers around a shared implementation.
Changes:
- Adds shared parsing, validation, execution, and failure aggregation.
- Restores the streamlined
slp_evalinterface. - Expands
measure_e2ewith scaling and latency controls.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/base.sh |
Adds shared benchmark implementation. |
scripts/slp_eval.sh |
Uses shared fixed evaluation configuration. |
scripts/measure_e2e.sh |
Adds configurable end-to-end latency benchmarking. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if ! is_nonnegative_integer "$BLOCK_TIME_MS" || [ "$BLOCK_TIME_MS" -le "$BLOCK_TIME_BAND_MS" ]; then | ||
| fail "Block time must be an integer greater than ${BLOCK_TIME_BAND_MS}ms." | ||
| fi |
There was a problem hiding this comment.
there's a special flag in core to ignore protocol-based bounds (4000ms min, 5000ms max), so this comment is wrong. Are we configuring the flag in this script though? It's called TESTING_IGNORE_LEDGER_TIME_UPGRADE_BOUNDS
There was a problem hiding this comment.
This flag gets auto-configured by min block.
supercluster/src/FSLibrary/StellarCoreCfg.fs
Lines 247 to 248 in f051651
supercluster/src/FSLibrary/MinBlockTimeTest.fs
Lines 355 to 357 in f051651
| return 1 | ||
| fi | ||
|
|
||
| existing=$((key_count / TIER1_ORG_SIZE)) |
| # Usage: sh scripts/measure_e2e.sh [SLP_EVAL_OPTION...] [-- MISSION_ARG...] | ||
| # This script runs the MinBlockTimeMixed mission with stellar-core's loadgen | ||
| # end-to-end latency metrics enabled, against the 2026-06-03 pubnet topology | ||
| # scaled out to 1000 nodes. It answers "how long does a transaction take to go |
There was a problem hiding this comment.
any reason to deviate from the original proposal here #394 (comment)? It uses survey topology without the scaling.
Make slp_eval and measure_e2e both easier to use wrappers around one common script.
slp_eval's interface is mostly restored to how it was pre 8351664.measure_e2e's interface is changed to make it easier to test against our scaling goals. Additionally, the following changes are made--ingress-external-hostis now a documented parameter for both scripts--enable-relaxed-auto-qset-configis added to the command line since Use automatic quorum set configuration by default #396 has still not mergedsac/oz/soroswap, the return value is 0 only iff all returned 0